Update README for fastapi integration #912
Conversation
|
Maybe that's overly complex, though? It also works to just to instantiate a client in some module, and use that in places where you need it, without the singleton class nor depends or anything such. If you don't need a connection pool or something. |
There was a problem hiding this comment.
Well, I kind of intentionally put this right below Module-level client so that this can be implemented as a dependency in FastAPI with a singleton approach.
We need a full-featured async client for our production server and I believe somebody else needs it as well.
If a complexity is a problem, I guess we could add a simplified example to this.
Ah sorry, I missed that it was there, was just reading the diff. I think this way it makes a lot of sense, to have the trivially simple option, and then this fully featured way. |
Actually, if you don't mind me asking, what's the need exactly? Your code seems to do a client per session, no? Why? I'm also using FastAPI and so far with just the client-in-module approach, and am not (yet) aware of problems it would introduce. A single client gets reused within a process, and is closed when the process shuts down. AFAIK reusing the same client for multiple sessions is fine. I'm not saying that there would not be problems, would just like to learn what exactly, and I guess that would be useful for the documentation too. |
|
Like @antont mentioned you should be able to share the same client between requests, is there a reason you set one up per-session? This then simplifies the code a lot, so it should look something like this: from contextlib import asynccontextmanager
from fastapi import FastAPI
from openai import AsyncOpenAI
client = AsyncOpenAI()
@asynccontextmanager
async def lifespan(app: FastAPI):
yield
await client.close()
app = FastAPI(lifespan=lifespan) |
There was a problem hiding this comment.
@RobertCraigie
Thank you for your comments.
But the class AioClient in the example is a wrapper to handle the state of a single client.
The wrapper class sets a new client, a prop of itself, when the server starts and returns the initialized
client when __call__ gets called, ie its instance (aio_client) gets called as a FastAPI's dependency.
The wrapper class will set a new client after initialization only if the client prop is None for some reason.
You can check the object identity of aio_client in async def foo with python's id() etc.
The reason why I included the wrapper class is that a wrapper makes it easier to handle states of multiple initialized clients
for multiple api keys etc AND fits into the dependency injection system of FastAPI.
If you want to simplify the code it is ok with me, but I guess it'd be better to add another example for dependency injection to the example above.
We use it since the same client is required for a bunch of routes and I believe it would be helpful
for most of FastAPI users.
If you are ok with this I will fix the proposed example in this PR so that two examples would be there for FastAPi.
from openai import AsyncOpenAI
from fastapi import Depends, FastAPI
client = AsyncOpenAI()
def get_client() -> AsyncOpenAI:
return client
app = FastAPI()
@app.get("/foo")
async def foo(client: AsyncOpenAI = Depends(get_client)) -> str:
res = await client.chat.completions.create(...)
...
return "ok"https://fastapi.tiangolo.com/tutorial/dependencies/
https://fastapi.tiangolo.com/advanced/events/
Right, figures. I'd think that a more complex example is useful, for example one that indeed gives different clients for different request handlers. The minimal case seems weird as it does not really have any benefit over just using a single client without the Dependency. The client supports using different API keys for different requests, though, with a single client, so I'm not sure what would be a good simple example case for a wrapper. Just my 2 cents, lib authors may see this differently. |
| https://github.com/tiangolo/fastapi/discussions/8301 | ||
| https://fastapi.tiangolo.com/advanced/events/ | ||
| """ | ||
| client: AsyncOpenAI | None = None |
There was a problem hiding this comment.
It's important to note that the use of | as a Union operator is supported starting from Python 3.10, as detailed in PEP 604. To ensure broader compatibility, especially with versions as early as Python 3.7, I recommend updating the code to use an alternative approach that aligns with the capabilities of these earlier Python versions
Changes being requested
Additional context & links
I guess
how to integrate with frameworks like FastAPIshould be documented to avoid confusion.#820